Skip to content

3310 optimise dependency tool#3311

Merged
arporter merged 4 commits intomasterfrom
3310_optimise_dependency_tool
Feb 5, 2026
Merged

3310 optimise dependency tool#3311
arporter merged 4 commits intomasterfrom
3310_optimise_dependency_tool

Conversation

@hiker
Copy link
Collaborator

@hiker hiker commented Jan 30, 2026

Delivers significant speedup in the dependency analysis for files that have many array indices that just use a simple reference as index (a(i,j), not a(i+1, h)) by avoiding calling sympy in these simple cases.

In two ukca files some loops are 30 to over 40 times faster (measurements in #3310).

@codecov
Copy link

codecov bot commented Jan 30, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.95%. Comparing base (854913c) to head (9a9815a).
⚠️ Report is 5 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3311   +/-   ##
=======================================
  Coverage   99.95%   99.95%           
=======================================
  Files         380      380           
  Lines       53912    53916    +4     
=======================================
+ Hits        53890    53894    +4     
  Misses         22       22           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@hiker
Copy link
Collaborator Author

hiker commented Jan 30, 2026

IT passed as well, a minor change with big impact (on selected few files ;) )

@hiker
Copy link
Collaborator Author

hiker commented Feb 2, 2026

I am taking this back for now. While one case (above) seems consistent, I re-did some measurements, and I am getting some inconclusive results ... it could be that the mn416-branch that I used missed some updates to the dependency_tools?

@hiker
Copy link
Collaborator Author

hiker commented Feb 3, 2026

I've verified that the 'speedup' of the DA on master was caused because the references were not resolved (turned up as unknown, so DA aborted early). Resolving the symbol fixes the 'speedup' (which marked the loop as not parallelisable, even though it is), it is then slow again with the correct result, and this patch fixes it:

master:
ukca_um_strat_photol_mod.F90	2	Execution time loop i:	14.372405	True	
ukca_um_strat_photol_mod.F90	3	Execution time loop j:	10.499314	True	
ukca_aerod.F90	                1	Execution time loop j:	35.180508	False	

3310:
ukca_um_strat_photol_mod.F90	2	Execution time loop i:	0.155515	True	
ukca_um_strat_photol_mod.F90	3	Execution time loop j:	0.132040	True	
ukca_aerod.F90	                1	Execution time loop j:	0.695516	False	

@hiker
Copy link
Collaborator Author

hiker commented Feb 3, 2026

Looks like no changes, so IT do not run again. Ready to go. Note that there are still some outliers when running the DA on ukca. I've added some additional potential improvements to #3183 (#3183 (comment))

Copy link
Member

@arporter arporter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All looks good to me.
Tests all OK. No need to update docs.
This is ready to go.

@arporter arporter merged commit 16639e7 into master Feb 5, 2026
15 checks passed
@arporter arporter deleted the 3310_optimise_dependency_tool branch February 5, 2026 16:37
@hiker hiker mentioned this pull request Feb 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants